Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Add rust-sdk position test #601

Open
wants to merge 17 commits into
base: main
Choose a base branch
from

Conversation

pauldragonfly
Copy link
Contributor

Add Position and Position Bundle Test Coverage

Overview

  • Introduces additional position and position bundle tests, aligned with the TypeScript SDK tests.

Changes

  • Implemented setup_position and setup_position_bundle for SPL and Token-2022.
  • Added tests for:
    • Position ownership and retrieval
    • Position bundle creation and retrieval

Notes

  • Tests are currently #[ignore] due to missing getProgramAccounts support in solana-bankrun.
  • Once support is added, these tests can be enabled by removing the #[ignore] attributes.

@pauldragonfly pauldragonfly marked this pull request as ready for review December 17, 2024 18:14
let ctx = RpcContext::new().await;

// Use token utility functions
let position_mint = setup_mint_with_decimals(&ctx, 0).await?;
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

You don't have to setup mint and ata when opening a position. The OpenPosition instruction will do that

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Updated to use OpenPosition ones!

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

You just need to derive the address and not actually set up the accounts yet. OpenPosition instruction will try to set up the accounts and if they are already set up it fails

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Ah understand what you mean. remove not to setup those things

setup_ata_with_amount(&ctx, mint_b, 1_000_000_000).await?;

let whirlpool = setup_whirlpool(&ctx, mint_a, mint_b, 64).await?;
let position_pubkey = setup_position(whirlpool).await?;
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Might be nice to add a position bundle and te_position as well to check if fetch_positions_for_owner finds those positions as well.

Same goes from the fetch_positions_in_pool test

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Added both to check

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Here it still only sets up the normal postion. Did you forget to commit?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Indeed, updated now

pub async fn setup_position(whirlpool: Pubkey) -> Result<Pubkey, Box<dyn Error>> {
let ctx = RpcContext::new().await;

let (position_pubkey, position_bump) = get_position_address(&Pubkey::new_unique())?;
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Right now the position_pubkey gets derived from a different mint than you supply in the OpenPosition instruction (which will result in a constraint violation).

In order to fix this:

let position_mint =  ctx.get_next_keypair();
let (position_pubkey, position_bump) = get_position_address(& position_mint.pubkey())?;

...

ctx.send_transaction_with_signers(vec![open_position_ix], vec![position_mint]).await?;

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I see, updated pattern to use this pattern

pub async fn setup_te_position(whirlpool: Pubkey) -> Result<Pubkey, Box<dyn Error>> {
let ctx = RpcContext::new().await;

let position_mint = Keypair::new();
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Same goes for the TE position as above. You don't have to init the mint and ata for the position.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Updated


// Use token utility functions
let position_bundle_mint = setup_mint_with_decimals(&ctx, 0).await?;
let position_bundle_token_account = setup_ata(&ctx, position_bundle_mint).await?;
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

And the same here. No need to init the mint and ata as InitializePositionBundle will do that

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Updated

position_mint: Pubkey::new_unique(),
position_token_account: Pubkey::default(),
position_mint: position_mint.pubkey(),
position_token_account: Pubkey::default(), // instruction will create
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The instruction will initiate the account but you still need to provide the correct address for position_token_account

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Updated!

whirlpool,
token_program: TOKEN_PROGRAM_ID,
token_program: TOKEN_PROGRAM_ID, // or TOKEN_2022_PROGRAM_ID if needed
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

TOKEN_2022_PROGRAM_ID is only for TE positions so irrelevant here

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Updated

let (position_pubkey, position_bump) = get_position_address(&te_position_mint.pubkey())?;

// 3) Build an OpenPosition instruction with token_program = TOKEN_2022_PROGRAM_ID
let position_token_account = get_associated_token_address_with_program_id(
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

There is one more for position bundle

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

my bad, updated

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants